Skip to content

fix: parse arguments to CardService.reorder correctly to int#7154

Merged
luka-nextcloud merged 1 commit into
nextcloud:mainfrom
vdiezel:fix-reorder-card-rest-endpoint
Sep 26, 2025
Merged

fix: parse arguments to CardService.reorder correctly to int#7154
luka-nextcloud merged 1 commit into
nextcloud:mainfrom
vdiezel:fix-reorder-card-rest-endpoint

Conversation

@vdiezel

@vdiezel vdiezel commented Aug 4, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

This correctly casts cardId and stackId (and order, just for consistency) into integers in CardService.reorder, no matter where CardService.reorder is called from. This fixes the call from CardApiController.reorder, because it passes cardId and stackId as strings. I've seen other places where there is no type cast applied, but it seems it is not an issue most of the time when no identity operator === or !== is used. Reorder does use it, so that is why it broke.

The type casting currently is a little bit inconsistent: There is everything from relying on php's implicit type casts from type defs in function parameters, to explicit type casting or none at all, but I was too afraid to touch it, as it might break things. So I kept my change minimal.

Tested this from the native client by modifying the URL in CardApi.reorder:

	reorderCard(card) {
		return axios.put(this.url(`/api/v1.0/boards/$YOUR_BOARD_ID_HARDCODED/stacks/${card.stackId}/cards/${card.id}/reorder`), card)
			.then(
				(response) => {
					return Promise.resolve(response.data)
				},
				(err) => {
					return Promise.reject(err)
				},
			)
			.catch((err) => {
				return Promise.reject(err)
			})
	}

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests (unit, integration, api and/or acceptance) are included
  • Documentation (manuals or wiki) has been updated or is not required

@vdiezel vdiezel force-pushed the fix-reorder-card-rest-endpoint branch from 245dc06 to 2116bb8 Compare August 4, 2025 17:54
grnd-alt
grnd-alt requested changes Aug 6, 2025
View reviewed changes

@grnd-alt grnd-alt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks a lot for your first contribution 🔥
I think the type definitions in cardService should definitely be there.
The type coercion though is only done in non strict mode (which is not set, but still) so I think we should keep the explicit type conversion we have in CardController right now, as it also doesn't have any downside afaik

@vdiezel vdiezel force-pushed the fix-reorder-card-rest-endpoint branch from 2116bb8 to 3a24dc4 Compare August 7, 2025 09:53
@vdiezel
fix: parse arguments to CardService.reorder correctly to int
eee35ac 
Signed-off-by: Viktor Diezel <viktor.diezel@posteo.de>
@vdiezel vdiezel force-pushed the fix-reorder-card-rest-endpoint branch from 3a24dc4 to eee35ac Compare August 7, 2025 09:55
@vdiezel

vdiezel commented Aug 7, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

thanks a lot for your first contribution 🔥 I think the type definitions in cardService should definitely be there. The type coercion though is only done in non strict mode (which is not set, but still) so I think we should keep the explicit type conversion we have in CardController right now, as it also doesn't have any downside afaik

Thank you!
I see. Then we also should not rely on it in the CardApiController.reorder. I added explicit conversion there, too.

@vdiezel vdiezel requested a review from grnd-alt August 14, 2025 09:32
@github-actions

github-actions Bot commented Aug 19, 2025

Copy link
Copy Markdown
Contributor

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

@luka-nextcloud luka-nextcloud merged commit 6a7c02d into nextcloud:main Sep 26, 2025
36 checks passed
@luka-nextcloud

luka-nextcloud commented Sep 26, 2025

Copy link
Copy Markdown
Contributor

/backport to stable31

@luka-nextcloud

luka-nextcloud commented Sep 26, 2025

Copy link
Copy Markdown
Contributor

/backport to stable32

@backportbot backportbot Bot mentioned this pull request Sep 26, 2025
Merged
@backportbot backportbot Bot mentioned this pull request Sep 26, 2025
Merged
@Olen Olen mentioned this pull request Nov 25, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@grnd-alt grnd-alt grnd-alt approved these changes
@luka-nextcloud luka-nextcloud Awaiting requested review from luka-nextcloud luka-nextcloud is a code owner
@elzody elzody Awaiting requested review from elzody

Assignees

No one assigned

Labels

feedback-requested

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Rest API: Reorder card not working / missing type casting for Ids

3 participants

@vdiezel @luka-nextcloud @grnd-alt